Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add support for projects_v2 and projects_v2_item webhook events #2868

Merged
merged 2 commits into from
Aug 14, 2023
Merged

Add support for projects_v2 and projects_v2_item webhook events #2868

merged 2 commits into from
Aug 14, 2023

Conversation

nikpivkin
Copy link
Contributor

Closes #2867

Added support for projects_v2 and projects_v2_item webhook events.

@nikpivkin nikpivkin marked this pull request as ready for review August 13, 2023 10:54
@codecov
Copy link

codecov bot commented Aug 13, 2023 •
edited
Loading

Codecov Report

Merging #2868 (1ecdb37) into master (899235e) will increase coverage by 0.00%.
Report is 1 commits behind head on master.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #2868   +/-   ##
=======================================
  Coverage   98.06%   98.07%           
=======================================
  Files         138      139    +1     
  Lines       12327    12346   +19     
=======================================
+ Hits        12089    12108   +19     
  Misses        162      162           
  Partials       76       76
Files Changed Coverage Δ
github/event_types.go 100.00% <ø> (ø)
github/messages.go 100.00% <ø> (ø)
github/event.go 100.00% <100.00%> (ø)

... and 2 files with indirect coverage changes

@gmlewis
Copy link
Collaborator

gmlewis commented Aug 13, 2023

This PR has significant conflicts with #2865 due to said PR dramatically changing the internals for webhook events (which will hopefully also reduce the maintenance burden and increase the simplicity of future PRs like this one).

So I apologize for the timing.

We can either move forward with this PR and have more work required in #2865 or we can move forward with #2865 then require changes here.

I'm honestly fine either way.

Do you have a preference @nikpivkin ?
Or do you have a preference, @evankanderson?

@evankanderson
Copy link
Contributor

I'm happy either way; I'll be updating my PR tomorrow probably

@gmlewis gmlewis changed the title feat: support for projects_v2 and projects_v2_item webhook events Add support for projects_v2 and projects_v2_item webhook events Aug 13, 2023
gmlewis
gmlewis requested changes Aug 13, 2023
View reviewed changes
Copy link
Collaborator

@gmlewis gmlewis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, @nikpivkin !
Just a few minor tweaks, please before proceeding.

github/event_types.go Outdated Show resolved Hide resolved
github/event_types.go Outdated Show resolved Hide resolved
github/event_types.go Outdated Show resolved Hide resolved
github/event_types_test.go Outdated Show resolved Hide resolved
@gmlewis gmlewis added the NeedsReview PR is awaiting a review before merging. label Aug 13, 2023
@nikpivkin nikpivkin requested a review from gmlewis August 13, 2023 16:29
gmlewis
gmlewis approved these changes Aug 13, 2023
View reviewed changes
Copy link
Collaborator

@gmlewis gmlewis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, @nikpivkin !
LGTM.

Awaiting second LGTM+Approval from any other contributor to this repo before merging.
(Maybe @evankanderson has time so that this can be merged before work is continued on #2865 ?)

@evankanderson
Copy link
Contributor

Not sure if my review/approval helps, but LGTM. Double checked that the types seem to match the documentation.

@gmlewis
Copy link
Collaborator

gmlewis commented Aug 14, 2023

Not sure if my review/approval helps, but LGTM. Double checked that the types seem to match the documentation.

Yes it does, as you are a valued contributor to this repo. Thank you, @evankanderson !
Merging.

@gmlewis gmlewis merged commit 3ecf12c into google:master Aug 14, 2023
9 checks passed
@nikpivkin nikpivkin deleted the feat/projects-v2-webhook branch August 15, 2023 04:34
evankanderson added a commit to evankanderson/go-github that referenced this pull request Aug 15, 2023
@evankanderson
Merge google#2868 and address gmlewis feedback
2d83b50
@gmlewis gmlewis removed the NeedsReview PR is awaiting a review before merging. label Aug 16, 2023
@gmlewis gmlewis mentioned this pull request Aug 16, 2023
Merged
gmlewis pushed a commit to gmlewis/go-github that referenced this pull request Sep 19, 2023
@nikpivkin @gmlewis
Add support for projects_v2 and projects_v2_item webhook events (goog…
7c1d2a6 
…le#2868)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@evankanderson evankanderson evankanderson approved these changes

@gmlewis gmlewis gmlewis approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Feature Request: support for projects_v2 and projects_v2_item webhook events
3 participants
@nikpivkin @gmlewis @evankanderson